Skip to content

Conversation

@achalddave
Copy link
Contributor

This isn't perfect, since it doesn't try to parse the yapf output, but it at least won't overwrite the file if there's an error.

@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@achalddave
Copy link
Contributor Author

Just signed the CLA.

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Nov 2, 2015
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you drop this reassignment while you're in here? AFAICT it serves no purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, didn't notice that. Done.

@achalddave
Copy link
Contributor Author

This is actually wrong, and I don't think I'm familiar enough with vimscript to fix it. Specifically, yapf returns error code 2 if the error is changed, which means I have to catch the ShellError exception, then check if the error code is 2, and do the same formatting logic as in the try. Something like

    try
      let l:result = maktaba#syscall#Create(l:cmd).WithStdin(l:input).Call()
    catch /ERROR(ShellError):/
      if v:shell_error == -1
        call maktaba#error#Shout('Error formatting file: %s', v:exception)
      endif
    endtry
    let l:formatted = split(l:result.stdout, "\n")

    call maktaba#buffer#Overwrite(1, line('$'), l:formatted)

doesn't work since l:result is scoped to within the try block... I don't know how to declare result to have scope outside the try block in vimscript.

Update: for now, I just got rid of the try catch and checked the v:shell_error variable manually.

yapf only returns an error code of 0 if it does not modify the file.
If it does modify the file, it returns error code 2, which causes
maktaba to throw a ShellError. Now, we just manually check the error
code instead of attempting to catch the exception.
@achalddave
Copy link
Contributor Author

Does this look good? I've been using it for the past few days and it seems to work fine.

@dbarnett
Copy link
Contributor

dbarnett commented Nov 7, 2015

Yep, looks good. Thanks!

dbarnett added a commit that referenced this pull request Nov 7, 2015
Fixes #61: Catch yapf errors instead of overwriting file
@dbarnett dbarnett merged commit 356212d into google:master Nov 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants